Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Viewfinder toggle #137

Merged
merged 6 commits into from
Oct 6, 2023
Merged

Viewfinder toggle #137

merged 6 commits into from
Oct 6, 2023

Conversation

danton267
Copy link
Collaborator

Let user disable/enable the viewfinder

todo in the future: once the user loading data is implemented, save the viewfinder toggle state and load it when the user returns. Eg when they disable it and come back, it should be automatically disabled

@danton267 danton267 self-assigned this Oct 5, 2023
@danton267 danton267 linked an issue Oct 5, 2023 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

Staging application has been deployed and is available at: https://dash5-services.plotly.host/ml-exchange-staging
Production app name: ml-exchange
Current branch name: viewfinder-toggle
Commit: 9363694

@cleaaum
Copy link
Collaborator

cleaaum commented Oct 5, 2023

Seems to be working perfectly well for me locally! ✨ However I had totally pictured the toggle to be on the top right corner of the screen and not in the menu bar! Kind of like the "Controls" button when the sidebar menu is closed. Perhaps we should ask later in our sync what they prefer! In my mind the sidebar is just to manipulate/visualise the dataset, so I wouldn't necessarily put an item in there that controls the viewfinder.

Copy link
Collaborator

@hannahker hannahker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality looks good to me! Just see UI comments before merging.

components/control_bar.py Outdated Show resolved Hide resolved
components/control_bar.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@hannahker hannahker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 💃

@hannahker hannahker merged commit db5b0fb into main Oct 6, 2023
4 checks passed
@hannahker hannahker deleted the viewfinder-toggle branch October 6, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow user to toggle viewfinder
3 participants